Skip to content

ARTESCA-15250 Handle stacked bars#835

Merged
bert-e merged 3 commits intodevelopment/1.0from
feature/ARTESCA-15250-stacked
Jul 16, 2025
Merged

ARTESCA-15250 Handle stacked bars#835
bert-e merged 3 commits intodevelopment/1.0from
feature/ARTESCA-15250-stacked

Conversation

@JeanMarcMilletScality
Copy link
Contributor

@JeanMarcMilletScality JeanMarcMilletScality commented Jul 10, 2025

Add logic to sum values for stacked bars to get maxValue
Add test and story

Next Step : Add a sorting callback function

@JeanMarcMilletScality
ARTESCA-15250 Handle stacked bars
d95d57c 
Add logic to sum values for stacked bars to get maxValue
Add test and story
@bert-e
Copy link
Contributor

bert-e commented Jul 10, 2025

Hello jeanmarcmilletscality,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options
name description privileged authored
/after_pull_request Wait for the given pull request id to be merged before continuing with the current one.
/bypass_author_approval Bypass the pull request author's approval ⭐
/bypass_build_status Bypass the build and test status ⭐
/bypass_commit_size Bypass the check on the size of the changeset TBA ⭐
/bypass_incompatible_branch Bypass the check on the source branch prefix ⭐
/bypass_jira_check Bypass the Jira issue check ⭐
/bypass_peer_approval Bypass the pull request peers' approval ⭐
/bypass_leader_approval Bypass the pull request leaders' approval ⭐
/approve Instruct Bert-E that the author has approved the pull request. ✍️
/create_pull_requests Allow the creation of integration pull requests.
/create_integration_branches Allow the creation of integration branches.
/no_octopus Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
/unanimity Change review acceptance criteria from one reviewer at least to all reviewers
/wait Instruct Bert-E not to run until further notice.
Available commands
name description privileged
/help Print Bert-E's manual in the pull request.
/status Print Bert-E's current status in the pull request TBA
/clear Remove all comments from Bert-E from the history TBA
/retry Re-start a fresh build TBA
/build Re-start a fresh build TBA
/force_reset Delete integration branches & pull requests, and restart merge process from the beginning.
/reset Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

@bert-e
Copy link
Contributor

bert-e commented Jul 10, 2025

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

@JeanMarcMilletScality JeanMarcMilletScality force-pushed the feature/ARTESCA-15250-stacked branch from 6e8cde5 to d95d57c Compare July 11, 2025 15:27
@bert-e
Copy link
Contributor

bert-e commented Jul 11, 2025

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

@bert-e
Copy link
Contributor

bert-e commented Jul 15, 2025

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

Copilot AI reviewed Jul 15, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for stacked bars by summing segment values to compute the maximum, sorting series by average when stacked, and wiring the stacked prop through the component, including a Storybook entry and tests.

  • Renamed getMaxValue to getMaxBarValue and implemented summation for stacked items.
  • Updated formatPrometheusDataToChartData to sort bar series by their average when stacked is true.
  • Extended Barchart component to accept a stacked prop, apply stackId, and added a new Story and tests for stacking.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
stories/BarChart/barchart.stories.tsx Added Stacked story with sample stacked data
src/lib/components/barchartv2/utils.ts Replaced getMaxValue with getMaxBarValue and added stacked logic and sorting
src/lib/components/barchartv2/utils.test.ts Updated tests for getMaxBarValue and added stacked bar sorting tests
src/lib/components/barchartv2/Barchart.component.tsx Integrated stacked prop into chart rendering and added stackId
src/lib/components/barchartv2/Barchart.component.test.tsx Added a test to verify stacked bar rendering
Comments suppressed due to low confidence (3)

src/lib/components/barchartv2/utils.ts:38

  • [nitpick] The variable name filterOutCategory suggests a boolean but holds an array of keys; consider renaming it to nonCategoryKeys or categoryKeys for clarity.
      const filterOutCategory = Object.keys(item).filter(

src/lib/components/barchartv2/Barchart.component.test.tsx:233

  • This test only verifies category labels but doesn’t confirm that bars are actually stacked (e.g., checking stackId on each bar or correct rendering order). Consider adding assertions to validate stacking behavior.
  it('should render stacked bars', () => {

src/lib/components/barchartv2/utils.ts:152

  • [nitpick] The logic to derive dataKey from label is duplicated in multiple places; extracting it into a shared helper would improve maintainability and reduce duplication.
    dataKey: bar.label.toLowerCase().replace(/\s+/g, ''),

ChengYanJin
ChengYanJin reviewed Jul 15, 2025
View reviewed changes
@bert-e
Copy link
Contributor

bert-e commented Jul 15, 2025

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

@bert-e
Copy link
Contributor

bert-e commented Jul 16, 2025

Waiting for approval

The following approvals are needed before I can proceed with the merge:

  • the author

  • one peer

Peer approvals must include at least 1 approval from the following list:

@JeanMarcMilletScality
Copy link
Contributor Author

JeanMarcMilletScality commented Jul 16, 2025

/approve

@bert-e
Copy link
Contributor

bert-e commented Jul 16, 2025

I have successfully merged the changeset of this pull request
into targetted development branches:

  • ✔️ development/1.0

Please check the status of the associated issue ARTESCA-15250.

Goodbye jeanmarcmilletscality.

The following options are set: approve

@bert-e bert-e merged commit f4e3717 into development/1.0 Jul 16, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@ChengYanJin ChengYanJin ChengYanJin approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@JeanMarcMilletScality @bert-e @ChengYanJin